docs+fixes(cni): finish the CNI plugin-chain split — docs, containerlab, dead code by privateip · Pull Request #307 · datum-cloud/galactic

privateip · 2026-08-08T05:10:13Z

Stack (merge bottom to top):

refactor(cni): split galactic-cni into veth/tap master plugins #303 refactor(cni): split galactic-cni into veth/tap master plugins
refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin #304 refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin
refactor(cni): extract galactic-bgp as its own CNI chain plugin #305 refactor(cni): extract galactic-bgp as its own CNI chain plugin
refactor(cni): extract galactic-route as its own CNI chain plugin #306 refactor(cni): extract galactic-route as its own CNI chain plugin
docs+fixes(cni): finish the CNI plugin-chain split — docs, containerlab, dead code #307 docs+fixes(cni): finish the CNI plugin-chain split - docs, containerlab, dead code (this PR)

Summary

Fifth and final branch in the CNI plugin-chain split stack (based on #306). Docs, containerlab fixtures, and leftover dead code brought up to date with the 5-binary chain steps 0-3 built. No new runtime behavior beyond the fixes below.

installer.Bootstrap's binary-copy list already covers all five chain binaries (added incrementally per step) — nothing left to do there. Confirmed the static host-level conflist only ever carries node-level settings, never per-attachment fields, so its shape needs no change.

Docs rewritten

docs/cni-cmd-sequence.md — all 3 diagrams rewritten with per-binary lifelines. Also fixes a pre-existing staleness: the diagrams showed a kernel seg6local route install, which the 2026-08-02 eBPF uSID datapath cutover had already replaced with vrf_table/locator_table/function_table map registration.
docs/cni/configuration.md — full rewrite: chain/conflist structure, per-binary field tables, all 7 example configs as multi-plugin conflists. Corrected two inaccuracies: IPAM.Routes/Addresses are declared but never read by any allocation path; pool IPAM's DEL no longer reads a BGPAdvertisement CRD annotation (superseded by on-disk marker files).
docs/agents/ARCHITECTURE.md — repo layout, components, entry points, config, module/package reference, key design decisions, testing, known constraints all updated. Same eBPF-cutover fix as the sequence doc. Added Known Constraints notes for two coverage gaps: e2e doesn't exercise galactic-route/galactic-bgp (pre-existing, unchanged by the split), and vmtap-cni (unrelated binary) has its own doc.

Real regression found and fixed: containerlab NAD fixtures

All 11 NAD YAML files under deploy/containerlab/ still used the pre-split single-plugin shape — no "ipam" block, no "plugins" chain array. Left as-is, every attachment would ADD successfully but silently allocate no IPAM address (top-level fields are unknown JSON to the new PluginConf) and never get BGP-advertised (no galactic-bgp invocation). Rewrote all 11 as plugins: [galactic-cni, galactic-bgp] conflists with addressing moved into an explicit ipam block. deploy/containerlab/docs/tenants.md's narrative updated to match.

e2e test fix

TestCNITapInterface invoked /galactic-cni directly with interface_type: tap and a bogus srv6_locator field — both removed in step 0, so it's been exercising nothing since. Fixed to invoke /galactic-tap-cni with the explicit-contract ipam shape and CNI_PATH=/ (where the image actually places every binary). Removed the eBPF-control-daemon startup this test no longer needs — eBPF registration is galactic-bgp's job now, which this test doesn't invoke (verifying that end-to-end needs a BGPRouter fixture, flagged in Known Constraints, not attempted here — no regression versus the monolithic predecessor's own coverage).

Dead code removed

cmd/galactic-cni/main.go: deleted the stdin-peek-and-repipe dance and isTapMode() — unreachable once galactic-tap-cni became its own binary. cmd/galactic-tap-cni/main.go's copy-pasted version simplified too (it already set CNI_NETNS_OVERRIDE unconditionally; the buffering around it inspected nothing).
internal/config/cni.go: removed GALACTIC_CNI_ENABLE_LOCAL_IPAM/CNIGetEnableLocalIPAM() — step 1 introduced the successor but never removed the predecessor; zero production callers remained.
Doc comments in internal/plumbing/ebpf/doc.go, usidmap/vrf.go, internal/plumbing/srv6/usid.go, internal/gc/gc.go still pointing at the deleted internal/cni/bgp.go — updated to internal/cnibgp/bgp.go.

Verification

task lint ✅ task build (all 8 binaries) ✅ task test:unit ✅
task test:e2e not run, same caveat as every prior PR in this stack.

🤖 Generated with Claude Code

mattdjenkinson · 2026-08-08T17:07:34Z

Reviewed this one, the last in the stack. Deleting the isTapMode/CNI_NETNS_OVERRIDE bypass here is the right move, that's the exact dead code I flagged back on #303, but removing it changes the failure mode rather than closing it cleanly, and it's worth a look before merge.

cmd/galactic-cni/main.go:108: the vendored CNI skel library runs funcs.Add to completion before it ever checks CNI_NETNS_OVERRIDE or same-netns, that check only fires after cmdAdd returns. And internal/cni/ops_add.go's deferred cleanup only runs when err is non-nil, never on success. So if a tap-shaped or misrouted config still reaches the now veth-only galactic-cni binary running in host netns (say, a stray interface_type: "tap" field, which json.Unmarshal now silently ignores since PluginConf has no such field), cmdAdd runs all the way through, creates a VRF, a veth pair, a host route, and a NAD annotation patch, returns nil, and only then does skel's post-hoc same-netns check reject the ADD from the runtime's point of view. Nothing rolls any of that back, so every retry leaks more. NAD generation itself lives in an external operator outside this repo, so there's no way to fully rule out a caller like this still existing somewhere mid-migration across the stack.

A few smaller things. The 11 rewritten NAD YAML fixtures under deploy/containerlab/resources/tenants/** are hand-duplicated near-identical copies, only the VPC/attachment IDs and subnet CIDRs differ, rather than generated off the Kustomize base/overlay mechanism the same tree already uses elsewhere. That's the same failure mode that produced the bug this PR fixes in the first place: the earlier config-shape change had to be hand-propagated across 11 files and drifted before anyone noticed. Nothing here stops that from happening again on the next shape change.

cmd/galactic-cni/main.go:100 has a stale comment left over from code this PR deleted. It still says "...both skel's blocking stdin read and the io.ReadAll below would hang forever," but the io.ReadAll(os.Stdin) call it's referring to, along with the io import, is gone. Not a runtime issue, just misleading documentation sitting in the exact function this PR touches.

And docs/agents/ARCHITECTURE.md:195 has new and modified markdown tables that aren't column-aligned, which CONVENTIONS.md calls out explicitly as a rule for every table in this repo's markdown files.

Everything else I checked came back clean: the CNIGetEnableLocalIPAM/isTapMode removal itself, the e2e binary path fixes, the comment-only fixes elsewhere, NAD field names and CIDR correctness, and import/naming/error-handling conventions, all verified against build, vet, tests, and the actual code.

The resource-leak-on-retry issue is the one worth taking seriously, since it's a real gap the stack leaves open even though the change that surfaces it is the correct one to make.

CNI_NETNS_OVERRIDE (blocking): galactic-route never entered any netns, so cmd/galactic-route/main.go assumed it never needed the stdin peek-and-repipe dance or CNI_NETNS_OVERRIDE that galactic-cni/ galactic-tap-cni use. That's true for veth-mode attachments, where CNI_NETNS points at the container's netns and differs from this process's own ambient (host) netns. It's false for tap-mode attachments: CNI_NETNS is deliberately set to the host's own root netns there (no per-VM netns exists), which equals this process's ambient netns, so skel's post-Add/Del same-netns check rejected every tap-mode ADD/DEL with terminations even though the route was already installed correctly. Now peeks stdin for interface_type the same way galactic-cni does and sets CNI_NETNS_OVERRIDE=true only for tap mode. CHECK on-link routes: checkTerminationRoutes unconditionally called net.ParseIP on Via and errored on nil, but Via is omitempty and assembleRoute (route.go) has a real branch that installs a valid on-link route for an empty Via, which cmdAdd installs fine. CHECK always failed with "invalid termination gateway" for those regardless. Restructured the match loop to treat an empty Via as looking for a gateway-less, device-scoped route instead of erroring immediately. Carried over byte for byte from the pre-split internal/cni/ops_check.go (also reachable via internal/cnitap), so this fixes the same bug there too by virtue of the code having moved. Docs: docs/cni/configuration.md still listed terminations as a galactic-cni/galactic-tap-cni field and showed it inline in the master's own JSON, which this PR's PluginConf split made wrong -- the master's slimmer struct silently drops the field on unmarshal, so an operator following the doc gets a silent no-op. Moved the field out of the master's Top-Level Fields table, reworded Termination Fields to attribute it to galactic-route's own conflist stanza (including that cmdDel is a no-op, not "deleted in reverse order"), and rewrote the worked example as a chained plugins array. Deferred per the review: the internal/cniroute/config.go:205 dead `if conf.PrevResult != nil` branch is copy-pasted across internal/cni, internal/cnitap, and internal/cnibgp too, predating this PR -- left for #307, which already scopes "dead code" cleanup for the chain split. Verification: task lint, task build (all 8 binaries), task test:unit all pass. task test:e2e not run, same caveat as #303/#304/#305 -- this repo has no root/CAP_NET_ADMIN available, and the existing checkTerminationRoutes tests already can't get past the vrf.TableID lookup without a real kernel VRF, so the on-link CHECK fix has no new automated regression test beyond what task test:e2e's Kind cluster would exercise. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rebased onto the updated refactor/cni-chain-4-installer-docs (which picked up PR #307's own review-feedback fix commit) after that branch's history moved out from under this one -- same ripple as #306 -> #307. Conflicts resolved: internal/cni/resource.go and internal/cnitap/ resource.go both had two independent changes touching the same struct/cleanup() region -- this PR's own removal of vrfCreated/VRF deletion (since the VRF is now shared per-VPC, not per-attachment, so a single attachment's rollback must never delete it), and PR #305/#306's unrelated addition of ipamDelegated/ipamType/ipamStdin for real IPAM-delegation rollback. Kept both: dropped vrfCreated and the VRF-delete step, kept the IPAM rollback fields and step, and kept this PR's fuller "why no VRF deletion" doc comment (it explains the shared-VRF reasoning more completely than the version already in these files). internal/cnibgp/bgp.go had one similar conflict at the registerEBPFDatapath call site -- resolved to drop the vpcAttachment argument (this PR's change) while keeping the ebpfPinDir package var (added by #305's own fix, for test injection) rather than reverting to the attach.PinDir literal this PR's diff predates. Also fixed one file this PR's diff never touched: internal/cnibgp/ resource_test.go didn't exist yet at this PR's original base -- it was added later by #305's own fix commit -- so its direct vrf.Add/ vrf.Delete/vrf.TableID(vpc, vpcAttachment) calls needed the same vpcAttachment-arg removal this PR already applied everywhere else, or the package wouldn't build. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree. tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…nadpatch Rebased onto the updated refactor/cni-chain-4-installer-docs (which picked up PR #307's own review-feedback fix commit) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311. Conflicts resolved: internal/cnibgp/bgp.go, ops_check.go, and resource.go each had an import-block conflict from this PR's internal/cni/crdnames -> internal/crdnames promotion landing on lines the current tree had already changed independently (PR #305's own fix commit moved cnibgp's eBPF pin-dir handling behind a package- level ebpfPinDir var in cnibgp.go, so bgp.go/ops_check.go/resource.go no longer import internal/plumbing/ebpf/attach directly the way this PR's diff -- authored before that -- expected). Resolved by applying just the crdnames rename to each file and leaving the attach import out, matching how the current tree already gets ebpfPinDir. Also fixed one file this PR's diff never touched: internal/cnibgp/ resource_test.go didn't exist yet at this PR's original base -- it was added later by #305's own fix commit -- so its own "go.datum.net/galactic/internal/cni/crdnames" import needed the same promotion this PR already applied everywhere else, or the package wouldn't build. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree, including the three promoted packages (internal/crdnames, internal/hostconf, internal/nadpatch). tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rebased onto the updated fix/vrf-shared-per-vpc-1-core (which picked up my own rebase-and-reconcile of that branch after PR #307 moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311 -> #312. Conflicts resolved: this PR drops the eBPF vrf_table rollback path (registerEBPFDatapath no longer returns a block to track, unregisterEBPFDatapath is deleted entirely, publishResult loses ebpfRegistered/ebpfBlock/ebpfArgument) since the vrf_table entry is now shared per (VPC, node) same as the BGPVRFInstance CRD, so a failed ADD must never unregister it. The current tree had independently refactored resourceTracker to embed publishResult (rather than copying its fields one-by-one) between this PR's original base and now, so I kept that embedding -- it still holds exactly this PR's two surviving fields (advertisementCreated, vrfInstanceCreated) once the eBPF fields are gone -- and added this PR's own vrfInstanceCreated-conditioned-on-OperationResultCreated behavior and nodeName field on top of it, along with this PR's fuller cleanup() doc comment (it explains the shared-VRF reasoning more completely than what was already there). Also kept ebpfPinDir (the package-level var #305's own fix added for test injection) over the attach.PinDir literal this PR's diff predates, matching the same resolution #311 needed one level up. internal/cnibgp/resource_test.go needed a full rewrite rather than a per-hunk merge: it didn't exist yet at this PR's original base either (same gap #311 hit) -- it was added by #305's own fix commit -- so this PR's diff shows the file as "new," and the version already in the tree still tested the old unconditional-vrfInstanceCreated/ ebpfRegistered design this PR removes. Took this PR's four tests wholesale (they're purpose-built for the new design) and adjusted their resourceTracker literals for the embedded-publishResult shape (publishResult: publishResult{vrfInstanceCreated: true} instead of a bare vrfInstanceCreated: true field, which the embedding makes illegal in a keyed literal). Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree. tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rebased onto the updated fix/vrf-shared-per-vpc-2-cnibgp (which picked up my own rebase-and-reconcile of that branch after fix/vrf-shared-per-vpc-1-core moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311 -> #312 -> #313. No conflicts: this PR only touches containerlab docs/scripts/ manifests, none of which overlapped with the Go source changes further up the stack. Applied cleanly. Verification: task lint, task build (all 8 binaries) pass on the rebased tree. No Go source changed in this PR, so task test:unit is unaffected. tests/e2e and the containerlab lab itself not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…nimaster Rebased onto the updated fix/cni-review-followups-doc-placement (which picked up my own rebase-and-reconcile of that branch after PR #307 moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #315 -> #316. Conflicts resolved: internal/cni/resource.go and internal/cnitap/ resource.go each had two independent changes touching the same resourceTracker/cleanup() region -- this PR's own extraction of the shared k8s-client-construction (newK8sClient/cniScheme) and interface+ VRF rollback (veth.Delete/tap.Delete + vrf.Delete) into internal/cnimaster's NewK8sClient/CleanupAttachment, and PR #305/#306's unrelated addition of ipamDelegated/ipamType/ipamStdin fields plus an IPAM-release rollback step (this PR's diff predates that feature entirely, same gap #311/#312/#315 each hit one level up). Kept both: call cnimaster.CleanupAttachment for the interface+VRF half (this PR's whole point), and kept the IPAM release step ahead of it, unchanged. internal/cnitap/ops_check.go had one similar import-only conflict (this PR drops the netlink/rest/ctrl/vrf imports cnimaster. CheckNodeLevelState/ProbeAPIServer/RunStatus now cover internally) -- its own IPAM CHECK delegation step (ipam.ExecCheck, same predates-this- PR gap) sat entirely outside the conflicted hunk and needed no resolution beyond keeping the "github.com/containernetworking/plugins/ pkg/ipam" import alive. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree, including the new internal/cnimaster package. tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

ecv · 2026-08-09T22:23:27Z

hmm red e2e. sec i think i see it

…nadpatch Rebased onto the updated refactor/cni-chain-4-installer-docs (which picked up PR #307's own review-feedback fix commit) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311. Conflicts resolved: internal/cnibgp/bgp.go, ops_check.go, and resource.go each had an import-block conflict from this PR's internal/cni/crdnames -> internal/crdnames promotion landing on lines the current tree had already changed independently (PR #305's own fix commit moved cnibgp's eBPF pin-dir handling behind a package- level ebpfPinDir var in cnibgp.go, so bgp.go/ops_check.go/resource.go no longer import internal/plumbing/ebpf/attach directly the way this PR's diff -- authored before that -- expected). Resolved by applying just the crdnames rename to each file and leaving the attach import out, matching how the current tree already gets ebpfPinDir. Also fixed one file this PR's diff never touched: internal/cnibgp/ resource_test.go didn't exist yet at this PR's original base -- it was added later by #305's own fix commit -- so its own "go.datum.net/galactic/internal/cni/crdnames" import needed the same promotion this PR already applied everywhere else, or the package wouldn't build. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree, including the three promoted packages (internal/crdnames, internal/hostconf, internal/nadpatch). tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…nimaster Rebased onto the updated fix/cni-review-followups-doc-placement (which picked up my own rebase-and-reconcile of that branch after PR #307 moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #315 -> #316. Conflicts resolved: internal/cni/resource.go and internal/cnitap/ resource.go each had two independent changes touching the same resourceTracker/cleanup() region -- this PR's own extraction of the shared k8s-client-construction (newK8sClient/cniScheme) and interface+ VRF rollback (veth.Delete/tap.Delete + vrf.Delete) into internal/cnimaster's NewK8sClient/CleanupAttachment, and PR #305/#306's unrelated addition of ipamDelegated/ipamType/ipamStdin fields plus an IPAM-release rollback step (this PR's diff predates that feature entirely, same gap #311/#312/#315 each hit one level up). Kept both: call cnimaster.CleanupAttachment for the interface+VRF half (this PR's whole point), and kept the IPAM release step ahead of it, unchanged. internal/cnitap/ops_check.go had one similar import-only conflict (this PR drops the netlink/rest/ctrl/vrf imports cnimaster. CheckNodeLevelState/ProbeAPIServer/RunStatus now cover internally) -- its own IPAM CHECK delegation step (ipam.ExecCheck, same predates-this- PR gap) sat entirely outside the conflicted hunk and needed no resolution beyond keeping the "github.com/containernetworking/plugins/ pkg/ipam" import alive. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree, including the new internal/cnimaster package. tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rebased onto the updated fix/vrf-shared-per-vpc-2-cnibgp (which picked up my own rebase-and-reconcile of that branch after fix/vrf-shared-per-vpc-1-core moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311 -> #312 -> #313. No conflicts: this PR only touches containerlab docs/scripts/ manifests, none of which overlapped with the Go source changes further up the stack. Applied cleanly. Verification: task lint, task build (all 8 binaries) pass on the rebased tree. No Go source changed in this PR, so task test:unit is unaffected. tests/e2e and the containerlab lab itself not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rebased onto the updated fix/vrf-shared-per-vpc-1-core (which picked up my own rebase-and-reconcile of that branch after PR #307 moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311 -> #312. Conflicts resolved: this PR drops the eBPF vrf_table rollback path (registerEBPFDatapath no longer returns a block to track, unregisterEBPFDatapath is deleted entirely, publishResult loses ebpfRegistered/ebpfBlock/ebpfArgument) since the vrf_table entry is now shared per (VPC, node) same as the BGPVRFInstance CRD, so a failed ADD must never unregister it. The current tree had independently refactored resourceTracker to embed publishResult (rather than copying its fields one-by-one) between this PR's original base and now, so I kept that embedding -- it still holds exactly this PR's two surviving fields (advertisementCreated, vrfInstanceCreated) once the eBPF fields are gone -- and added this PR's own vrfInstanceCreated-conditioned-on-OperationResultCreated behavior and nodeName field on top of it, along with this PR's fuller cleanup() doc comment (it explains the shared-VRF reasoning more completely than what was already there). Also kept ebpfPinDir (the package-level var #305's own fix added for test injection) over the attach.PinDir literal this PR's diff predates, matching the same resolution #311 needed one level up. internal/cnibgp/resource_test.go needed a full rewrite rather than a per-hunk merge: it didn't exist yet at this PR's original base either (same gap #311 hit) -- it was added by #305's own fix commit -- so this PR's diff shows the file as "new," and the version already in the tree still tested the old unconditional-vrfInstanceCreated/ ebpfRegistered design this PR removes. Took this PR's four tests wholesale (they're purpose-built for the new design) and adjusted their resourceTracker literals for the embedded-publishResult shape (publishResult: publishResult{vrfInstanceCreated: true} instead of a bare vrfInstanceCreated: true field, which the embedding makes illegal in a keyed literal). Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree. tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rebased onto the updated fix/vrf-shared-per-vpc-2-cnibgp (which picked up my own rebase-and-reconcile of that branch after fix/vrf-shared-per-vpc-1-core moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311 -> #312 -> #313. No conflicts: this PR only touches containerlab docs/scripts/ manifests, none of which overlapped with the Go source changes further up the stack. Applied cleanly. Verification: task lint, task build (all 8 binaries) pass on the rebased tree. No Go source changed in this PR, so task test:unit is unaffected. tests/e2e and the containerlab lab itself not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

ecv

Approving — #333 is fixed here. All three binaries set CNI_NETNS_OVERRIDE unconditionally now, isTapMode and the peek-and-repipe are gone from galactic-route, and the comments say why rather than restating what.

CI is still running on this head; nothing in the change should trouble it.

I'll close #333 once this merges. #326 and #327 stay open for the parts noted above.

…ab, dead code Rebased onto the updated refactor/cni-chain-3-galactic-route (which picked up PR #306's own review-feedback fix commit, fe72a1e) after that branch's history moved out from under this one. Conflicts resolved: - All 11 containerlab tenant NAD manifests: kept this PR's plugins-array chain wrapper (adding the galactic-bgp stage) around the nested "ipam": {"type": "galactic-ipam", ...} block that PR #305's own fix commit had already introduced independently of this PR -- the two changes were orthogonal, so the merge is additive. - docs/cni/configuration.md: took this PR's fuller rewrite throughout (it supersedes PR #306's narrower doc fix -- e.g. this PR already covers the interface_type/terminations field removal and the renamed GALACTIC_IPAM_ENABLE_LOCAL_IPAM env var more completely), but preserved two things #306 fixed that this PR's diff predates and doesn't otherwise cover: the cniVersion 1.0.0/1.1.0 prevResult constraint paragraph, and "on-link route" (not "link-local route" -- fd01::/48 in the example isn't a link-local address) in the terminations example. - tests/e2e/e2e_test.go: this PR's diff removed startEBPFControlDaemon (call, definition, and the attach import) on the theory that TestCNITapInterface never touches the eBPF datapath. That was true when this PR's diff was authored, but PR #305's own fix commit (7cf773a) had independently added testChainedGalacticBGP, chaining galactic-bgp after the tap master's ADD and asserting BGPVRFInstance/ BGPAdvertisement CRD creation -- and registerEBPFDatapath's usidmap.OpenPinnedRegistry only opens already-pinned maps, it never loads/pins the eBPF program itself, so testChainedGalacticBGP can't succeed without startEBPFControlDaemon having run first. Restored the call, its definition, and the import, and updated the doc comments this PR had already rewritten (which claimed the test "does not chain into ... galactic-bgp") to describe the merged reality instead. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree. tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

TestCNITapInterface now chains galactic-bgp after the tap master plugin, and galactic-bgp registers the eBPF uSID datapath. Pinning its maps needs the node's real bpffs visible inside the test pod, so the pod override must carry the same bpf-fs hostPath volume and mount that config/cni/daemonset.yaml uses in production. Without it the test fails with: start eBPF uSID datapath: attach: create bpf map pin directory "/sys/fs/bpf/galactic": mkdir /sys/fs/bpf/galactic: no such file or directory Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Tap attachments pass the host netns, which triggers the CNI library's same-netns rejection check. galactic-tap-cni sets the override but the three chained plugins it delegates to did not: - galactic-ipam had no override at all - galactic-bgp claimed it didn't need one (only makes k8s API calls) - galactic-route tried to detect tap mode via interface_type, which was removed from chain plugin configs in #303 All three now set CNI_NETNS_OVERRIDE unconditionally. The override is a no-op for veth-mode (where CNI_NETNS differs from the ambient netns) and required for tap-mode (where they match). Fixes #333

Rebased onto the updated fix/vrf-shared-per-vpc-2-cnibgp (which picked up my own rebase-and-reconcile of that branch after fix/vrf-shared-per-vpc-1-core moved out from under it) after that branch's history moved out from under this one -- same ripple as #306 -> #307 -> #311 -> #312 -> #313. No conflicts: this PR only touches containerlab docs/scripts/ manifests, none of which overlapped with the Go source changes further up the stack. Applied cleanly. Verification: task lint, task build (all 8 binaries) pass on the rebased tree. No Go source changed in this PR, so task test:unit is unaffected. tests/e2e and the containerlab lab itself not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…nadpatch Rebased onto the updated refactor/cni-chain-4-installer-docs (which picked up PR #307's own review-feedback fix commit) after that branch's history moved out from under this one -- same ripple as Conflicts resolved: internal/cnibgp/bgp.go, ops_check.go, and resource.go each had an import-block conflict from this PR's internal/cni/crdnames -> internal/crdnames promotion landing on lines the current tree had already changed independently (PR #305's own fix commit moved cnibgp's eBPF pin-dir handling behind a package- level ebpfPinDir var in cnibgp.go, so bgp.go/ops_check.go/resource.go no longer import internal/plumbing/ebpf/attach directly the way this PR's diff -- authored before that -- expected). Resolved by applying just the crdnames rename to each file and leaving the attach import out, matching how the current tree already gets ebpfPinDir. Also fixed one file this PR's diff never touched: internal/cnibgp/ resource_test.go didn't exist yet at this PR's original base -- it was added later by #305's own fix commit -- so its own "go.datum.net/galactic/internal/cni/crdnames" import needed the same promotion this PR already applied everywhere else, or the package wouldn't build. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree, including the three promoted packages (internal/crdnames, internal/hostconf, internal/nadpatch). tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…nimaster Rebased onto the updated fix/cni-review-followups-doc-placement (which picked up my own rebase-and-reconcile of that branch after PR from under this one -- same ripple as #306 -> #307 -> #315 -> #316. Conflicts resolved: internal/cni/resource.go and internal/cnitap/ resource.go each had two independent changes touching the same resourceTracker/cleanup() region -- this PR's own extraction of the shared k8s-client-construction (newK8sClient/cniScheme) and interface+ VRF rollback (veth.Delete/tap.Delete + vrf.Delete) into internal/cnimaster's NewK8sClient/CleanupAttachment, and PR #305/#306's unrelated addition of ipamDelegated/ipamType/ipamStdin fields plus an IPAM-release rollback step (this PR's diff predates that feature entirely, same gap #311/#312/#315 each hit one level up). Kept both: call cnimaster.CleanupAttachment for the interface+VRF half (this PR's whole point), and kept the IPAM release step ahead of it, unchanged. internal/cnitap/ops_check.go had one similar import-only conflict (this PR drops the netlink/rest/ctrl/vrf imports cnimaster. CheckNodeLevelState/ProbeAPIServer/RunStatus now cover internally) -- its own IPAM CHECK delegation step (ipam.ExecCheck, same predates-this- PR gap) sat entirely outside the conflicted hunk and needed no resolution beyond keeping the "github.com/containernetworking/plugins/ pkg/ipam" import alive. Verification: task lint, task build (all 8 binaries), task test:unit all pass on the rebased tree, including the new internal/cnimaster package. tests/e2e not run in this sandbox (no Kind cluster / root), same caveat as every PR in this stack. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

privateip requested a review from a team as a code owner August 8, 2026 05:10

privateip requested review from slindseysr and removed request for a team August 8, 2026 05:10

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from a0b9606 to 7d64f71 Compare August 8, 2026 12:03

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from 7d64f71 to 77169b0 Compare August 8, 2026 12:18

privateip mentioned this pull request Aug 8, 2026

Galactic CNI: publish per-pod SRv6 SID + tenant identifier via EndpointSlice for VPC HTTP ingress backends datum-cloud/enhancements#854

Closed

8 tasks

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from 77169b0 to f780964 Compare August 8, 2026 12:29

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from f780964 to 725ef87 Compare August 8, 2026 12:38

privateip requested review from 0xmc and scotwells August 8, 2026 12:44

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from 725ef87 to c690b5b Compare August 8, 2026 19:04

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from c690b5b to 6fdfbda Compare August 9, 2026 20:45

privateip requested a review from ecv August 10, 2026 18:20

ecv approved these changes Aug 10, 2026

View reviewed changes

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from afc568b to 8f0da2e Compare August 10, 2026 18:27

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from 8f0da2e to 1f03d1c Compare August 10, 2026 18:28

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from 1f03d1c to 2acb50a Compare August 10, 2026 18:44

ecv approved these changes Aug 10, 2026

View reviewed changes

Base automatically changed from refactor/cni-chain-3-galactic-route to main August 10, 2026 18:53

privateip and others added 3 commits August 10, 2026 14:53

privateip force-pushed the refactor/cni-chain-4-installer-docs branch from 2acb50a to 9e24f70 Compare August 10, 2026 18:53

privateip merged commit a2b88e9 into main Aug 10, 2026
8 checks passed

privateip deleted the refactor/cni-chain-4-installer-docs branch August 10, 2026 18:54

ecv mentioned this pull request Aug 10, 2026

Three leftovers from the CNI chain split #326

Closed

Navigation Menu

Uh oh!

docs+fixes(cni): finish the CNI plugin-chain split — docs, containerlab, dead code - #307

docs+fixes(cni): finish the CNI plugin-chain split — docs, containerlab, dead code#307
privateip merged 3 commits into
mainfrom
refactor/cni-chain-4-installer-docs

privateip commented Aug 8, 2026 •

edited

Loading

Uh oh!

mattdjenkinson commented Aug 8, 2026

Uh oh!

ecv commented Aug 9, 2026 •

edited

Loading

Uh oh!

ecv left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

privateip commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Docs rewritten

Real regression found and fixed: containerlab NAD fixtures

e2e test fix

Dead code removed

Verification

Uh oh!

mattdjenkinson commented Aug 8, 2026

Uh oh!

ecv commented Aug 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ecv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

privateip commented Aug 8, 2026 •

edited

Loading

ecv commented Aug 9, 2026 •

edited

Loading